Skip to content

Add ListInParaSniff to flag lists wrapped in a <para>#17

Open
lacatoire wants to merge 4 commits into
php:mainfrom
lacatoire:feat/list-in-para-sniff
Open

Add ListInParaSniff to flag lists wrapped in a <para>#17
lacatoire wants to merge 4 commits into
php:mainfrom
lacatoire:feat/list-in-para-sniff

Conversation

@lacatoire

Copy link
Copy Markdown
Member

Adds a sniff that flags list elements (simplelist, variablelist, itemizedlist, orderedlist) nested directly inside a <para>. In the PHP manual these belong as a direct child of the containing section, and wrapping them in a <para> is a recurring style/build issue.

It checks the list's immediate parent, so a list nested deeper (e.g. inside a <note>) is not flagged. An additionalListElements option allows extending the set. Registered in docbookcs.xml.dist.

Open questions for you: is the element set right (should <table>/<informaltable> be included too?), and is error the right default severity?

@jordikroon

jordikroon commented Jun 23, 2026

Copy link
Copy Markdown
Member

This is a good sniff to add, however it's too aggressive.
A violation should only trigger if:

  • It's a list or table
  • It's the only child element
  • No text

For example the following is allowed.

 <para>
   The following constants are defined, and represent possible
   <function>exif_imagetype</function> return values:
   <table>
     <title>Imagetype Constants</title>
     ...
   </table>
 </para>

When building new sniffs it's important to measure performance. To get a more detailed measurement, you can use the --perf parameter.

Some things that we may consider here depending on what gives the least overhead.

  • Use XPath (//path/table)
  • or: get all table nodes first, and then check if it's parent is a <para>.

XPath would be the cleanest IMHO as it avoids a nested loop.

Flag a list or table only when it is the sole child of the <para> and the
<para> carries no text of its own, so an intro sentence followed by a
<table> stays allowed. Add <table>/<informaltable> to the set and select
candidates with a single XPath pass instead of a nested loop.
An element name containing an apostrophe or any non-NCName character would
be injected verbatim into the XPath predicate, making the query invalid and
silently disabling the whole sniff. Drop such values and keep the defaults.
@alfsb

alfsb commented Jul 13, 2026

Copy link
Copy Markdown
Member

As commented, it is probably way faster to XPath search all "child" elements first, without //*[local-name()='para'], and then directly inspecting $node->parentNode->nodeName for "para"s. Simple searches and neighbour navigation is incredibly fast on libxml.

@lacatoire

lacatoire commented Jul 15, 2026

Copy link
Copy Markdown
Member Author

I ran the benchmark on the full doc-en reference directory (10 837 files) with both approaches:

  • para → children (current): 0.008s
  • elements → parentNode (your suggestion): 0.020s

The current XPath turns out to be ~2.5x faster, likely because there are far fewer nodes with direct block children to inspect than total occurrences of the disallowed elements across the whole tree.

@alfsb

alfsb commented Jul 15, 2026

Copy link
Copy Markdown
Member

Interessing. And go with the faster alternative.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants